fix(server): thread.turn.start bootstrap parity between websocket and http dispatch - #7996
fix(server): thread.turn.start bootstrap parity between websocket and http dispatch#7996pablospe wants to merge 6 commits into
Conversation
POST /api/orchestration/dispatch passed commands straight to the orchestration engine, skipping the bootstrap steps the WebSocket path runs for a thread.turn.start carrying `bootstrap`. An HTTP turn start for a not-yet-existing thread therefore failed with "Thread does not exist" even though the contract advertises bootstrap on that endpoint. The bootstrap flow (thread.create, worktree preparation, setup script, rollback via thread.delete on failure) now lives in a shared TurnStartBootstrap service that both the WebSocket RPC and the HTTP dispatch route use. The ws path keeps its client-origin stamping and command gating; HTTP gains the bootstrap branch. Tests cover HTTP bootstrap success and rollback alongside the existing ws cases.
github-actions
Bot
added
vouch:unvouched
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the new TurnStartBootstrap service and its call sites against the Effect service conventions. Service definition (namespace imports, inline Context.Service interface, make + layer, dependencies acquired via yield*), layer wiring in server.ts/ws.ts, and the HTTP dispatch path all look correct. One change-discipline issue on the moved helper is noted inline.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This change enables HTTP callers to create threads, prepare worktrees, launch setup scripts, and start turns through a new shared bootstrap workflow, with rollback and new error signaling. Its cross-transport orchestration and substantial side effects exceed a small, isolated fix and merit human review. You can add or adjust custom eligibility rules. Learn more. |
The move dropped the original never-typed default branch, so a new ProjectSetupScriptRunnerError tag would silently stringify into a user-visible activity payload instead of failing the build. Restored verbatim from the pre-move ws.ts helper.
pablospe
changed the title
The comment stayed behind in ws.ts above an unrelated function when the helper moved into TurnStartBootstrap.
Over WebSocket a failed bootstrap returns bootstrapThreadDisposition "deleted" so the client knows the thread is gone; the HTTP branch collapsed every failure into orchestration_dispatch_failed. It now maps that case to its own reason, orchestration_bootstrap_rolled_back, so an HTTP caller can retry with a fresh thread id instead of guessing. The origin-threading comment also stops claiming HTTP callers stamp one.
1 participant
What Changed
The thread-bootstrap flow that
thread.turn.startcan carry (bootstrap: create thread → prepare git worktree → run project setup script → start the turn, with rollback viathread.deleteon failure) moves verbatim out ofws.tsinto a sharedTurnStartBootstrapservice (apps/server/src/orchestration/TurnStartBootstrap.ts). Both transports now use it:ws.tscalls the service with its client origin, exactly as before; ~300 duplicated lines removed.orchestration/http.tsbranches to the service when athread.turn.startcarriesbootstrap. A bootstrap that fails after creating its thread (and rolls it back) now surfaces over HTTP as its own reason,orchestration_bootstrap_rolled_back— the HTTP equivalent of the WebSocket error'sbootstrapThreadDisposition: "deleted"— so a caller knows not to reuse the thread id.server.tsprovides the layer;bin.test.tsgets a mock layer.Tests (
vp test run apps/server/src/server.test.ts): HTTP bootstrap success asserts the command sequencethread.create → thread.meta.update → thread.turn.startwithbootstrapstripped from the final command and the worktree path propagated; failure asserts the rollbackthread.create → thread.deleteand the rolled-back reason. Existing WebSocket bootstrap tests untouched and green.Most of the diff is the moved block; net-new logic is the ~25-line HTTP branch and wiring.
Why
POST /api/orchestration/dispatchaccepts theClientOrchestrationCommandunion, whosethread.turn.startincludes the optionalbootstrapfield — but only the WebSocket path ran the bootstrap steps. Over HTTP the same command reached the engine untouched and failed withOrchestrationCommandInvariantError: Thread does not exist: the endpoint advertised a shape it broke on.This matters for anything driving a T3 server over HTTP (scripts, orchestrators): they had to reimplement bootstrap client-side — shelling
git worktree addlocally (same-machine only), hardcoding the worktree layout, and skipping the project setup script entirely. Moving the existing flow behind a shared service is transport parity, not new capability: the identical bearer auth already triggered this exact flow over WebSocket.Checklist
Note
Medium Risk
Touches command dispatch, git worktree creation, and thread rollback on both transports. Behavior is extracted rather than rewritten, but HTTP callers now trigger that path.
Overview
HTTP
POST /api/orchestration/dispatchnow runs the samethread.turn.startbootstrap as WebSocket: create thread, prepare worktree, optional setup script, then start the turn. Previously HTTP forwardedbootstrapto the engine and failed because the thread did not exist.The flow is extracted from
ws.tsinto a sharedTurnStartBootstrapservice. WebSocket still passes client origin on sub-commands; HTTP does not. If bootstrap fails afterthread.create, rollback still deletes the thread. HTTP surfaces that asorchestration_bootstrap_rolled_backso callers must not reuse the thread id.Tests cover HTTP success (
thread.create→thread.meta.update→ strippedthread.turn.start) and rollback (thread.create→thread.delete).Reviewed by Cursor Bugbot for commit 9dd69ae. Bugbot is set up for automated code reviews on this repo. Configure here.